Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add spec for after hook following before(:all) #2652

Closed
wants to merge 1 commit into from
Closed

Add spec for after hook following before(:all) #2652

wants to merge 1 commit into from

Conversation

sliaquat
Copy link

This is to confirm that after hook is not fired on failure in
before(:all) hook

Add spec for after hook following before(:all)
fcae9f0 
This is to confirm that after hook is not fired on failure in
before(:all) hook
@pirj
Copy link
Member

pirj commented Jul 30, 2019

What if in the subsequent hook we set up something that needs to be cleaned in after(:all) hook, e.g. create a file? How to figure out which after hooks need to be run, and which can be skipped?

@JonRowe
Copy link
Member

JonRowe commented Jul 31, 2019

@pirj the first before hook that fails will stop the entire run

pirj
pirj approved these changes Oct 13, 2019
View reviewed changes
Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

spec/rspec/core/hooks_spec.rb
@@ -145,6 +145,34 @@ def hook_collection_for(position, scope)
end
end

describe "#before(:all)" do
it "stops running subsequent after hook when an error is encountered" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd put this next to it "runs subsequent hooks of the same type when an error is encountered so all cleanup can complete" do.

JonRowe
JonRowe requested changes Oct 14, 2019
View reviewed changes
Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I'm not sure what this is trying to illustrate, can you elaborate what you'd like to achieve here @sliaquat?

@JonRowe
Copy link
Member

JonRowe commented Dec 18, 2019

Closing unless further is heard!

@JonRowe JonRowe closed this Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonRowe JonRowe JonRowe requested changes

@pirj pirj pirj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sliaquat @pirj @JonRowe